Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugging fixes #7038

Merged

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Feb 7, 2024

Late as always, but this time at least not affecting the IDE operation ....

During debugging of #7037, I've realized that debug flag was dropped during December from gradle tooling classes injected into gradle daemon. Debug info is needed for debugging and is very useful in stacktraces of user reports; I believe this change should go to the NB21 to provide better diagnostics.

The second fix is the name of the evn variable that NBLS launcher reads to inject extra options to the NBLS server. We used dotted name for historical reasons, but it seems it should be changed so that it is easier to set it up from the shell launcher. Dotted env var name requires use of env to launch the process, which is not always possible, i.e. in vscode remote development scenario that does a ssh login.

None of the changes affect normal IDE or NBLS operation. Please consider to include it into RC3, if not being already built.

@sdedic sdedic added Gradle [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests labels Feb 7, 2024
@sdedic sdedic added this to the NB21 milestone Feb 7, 2024
@sdedic sdedic self-assigned this Feb 7, 2024
Copy link
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

Should it have a debuglevel? I think this builds with all debug info, rather than the previous (source,lines) setting? Although I think that's the same as the IDE as a whole?!

@sdedic
Copy link
Member Author

sdedic commented Feb 7, 2024

IMHO if it builds with local variables, it is easier to attach to the daemon without recompilation for inspection. Also note that the gradle buildscript is not much used in the build, since in Dec 2023, the gradle build was replaced with ant build.

@neilcsmith-net
Copy link
Member

Sure, no problem with that - I was just comparing with the pre-Dec 2023 behaviour.

Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@neilcsmith-net neilcsmith-net merged commit 1ad003c into apache:delivery Feb 7, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gradle [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants